This repository has been archived by the owner on Sep 4, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 96
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…project file. When reverted back to internal on functions/classes
…References.cs. Removed dependencies for Mono.Addins.MSBuild on Mono.Addins.Setup. Not needed
MultiAssemblyAddin project has two projects in its directory and the SDK style file globs were including .cs files from these projects causing compilation errors. These project directories are now excluded from the project.
The path was being created without the path separator for the Mono.Addins.SetupProcess assembly. Also the file extension was incorrectly set to .dll when running with Mono.
The project was hard coded to target .NET 4.7.2 instead of using the TargetFramework.props file. This was preventing it being referenced by the UnitTests project.
The tests do not run tests with the external SetupProcess currently but having the UnitTests copy this allows this to be tested locally by changing the AddinDatabase.GetSetupHandler method.
Fixed the setup process tool trying to use too many command line arguments. The out directory is passed on stdin not as a command line argument.
Still have warnings about conflicts in mautil. One warning about Mono.Addins not being strong named.
Test addins now built into the lib directory instead of a subdirectory based on the target framework. UnitTests project now built without appending its target framework to the build directory. This fixes some tests that were relying on the location of the test addins being in the lib directory. Still 38 tests failing.
Some tests were checking the assembly version of the test addins were 0.0.0.0 however the default for SDK style projects seems to be 1.0.0.0. Changed the test addin projects to use the 0.0.0.0 version.
With the non-SDK style projects the DEBUG symbol is not defined so the Debug.Assert is not called. The Debug.Assert looks wrong. It is asserting that the dictionary contains the RuntimeAddin but that is always false when initially loading the dictionary. Commented this line out.
This was referenced Feb 9, 2021
It was targeting the .NET Framework.
@mrward Is there anything I can do ? |
There are still items on the TODO list which I will look at some point unless someone is keen to look at them. Right now I have done enough to start looking at using this .NET Standard branch in VS Mac. After investigating this I will get back to addressing the other problems at some point. |
Set AppendTargetFrameworkToOutputPath to false in all projects so everything builds into the same bin directory.
Fix warnings about assemblies not being strong named. The internals visible to information was missing the public key.
Before the UnitTests was using a ProjectReference without ReferenceOutputAssembly set to false. This would prevent the project building if the SetupProcess project was targeting an incompatible target framework, such as .NET Core. Now ReferenceOutputAssembly is set to false on the project reference and the executable files are copied in a custom msbuild target after the build finishes. Also have to add SkipGetTargetFrameworkProperties to true for the project reference to workaround an MSBuild bug where the build fails with UnitTests referencing a .NET Core 3.1 SetupProcess project even though it does not compile against its assemblies.
The original logic tried to run the dll directly which did not work. Now a check is made to see if the .exe is available and that is run if it is available. Then a check is made to see if there is a native build of the exe available. Finally the code falls back to running the dll with dotnet.
Therzok
reviewed
Jun 7, 2021
@@ -626,8 +626,8 @@ void RegisterAssemblyResolvePaths (RuntimeAddin addin, ModuleDescription descrip | |||
{ | |||
lock (LocalLock) { | |||
foreach (var asm in description.AssemblyNames) { | |||
Debug.Assert(assemblyResolvePaths[asm] == addin); | |||
assemblyResolvePaths[asm] = addin; | |||
//Debug.Assert(assemblyResolvePaths[asm] == addin); This does not look right. Not called in old project since DEBUG symbol is not defined. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure anymore, but I think the assert was supposed to pick up scenarios where there were multiple registrations of the same assembly.
Therzok
approved these changes
Jun 7, 2021
Those can't be built on .net standard. Leave them targetting 4.7 for now
Looking forward to this. Thank you |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on #169 and includes extra fixes
TODO: